Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@package.json`:
- Line 3: The npm dist-tags used in the publish scripts are out of sync with the
package version 1.0.0-alpha.18; update the publish commands so the dist-tag
passed to npm (references: the "publish:contracts" and "publish:sdk" script
entries in package.json) uses "alpha.18" (or derive from the package.json
version) instead of "alpha.17"; locate the "publish:contracts" and "publish:sdk"
script values and replace the "--tag alpha.17" token with "--tag alpha.18" (or
refactor to compute the tag from the version string) so published artifacts and
release metadata remain consistent.

---

Nitpick comments:
In `@sdk/typescript/utils/metaTx/metaTransaction.tsx`:
- Around line 64-65: The exported buildTypedDataMessage should return a concrete
typed shape instead of Record<string, unknown>; define a TypeScript interface
(e.g., MetaTransactionTypedData or EIP712MetaTransaction) that matches the
EIP-712 domain/types/message schema used by the Solidity contract, change
buildTypedDataMessage(metaTx: MetaTransaction): MetaTransactionTypedData, and
update signMetaTransactionWithWallet to accept/sign that typed payload (remove
the any cast) so the compiler enforces the fields; reference the functions
buildTypedDataMessage and signMetaTransactionWithWallet and the new interface
name when making these changes.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/sync-contract-versions.yml:
- Around line 76-85: The workflow currently only checks and commits changes to
contracts/core/lib/EngineBlox.sol's VERSION constant, but npm run
release:sync-versions also rewrites sdk/typescript/lib/EngineBlox.tsx
(EngineBlox.VERSION), so update the job to detect and commit both files
together: modify the diff/grep check that looks for constant VERSION in
EngineBlox.sol to also include sdk/typescript/lib/EngineBlox.tsx (or check for
VERSION in either file), and ensure the git add/git commit step stages both
contracts/core/lib/EngineBlox.sol and sdk/typescript/lib/EngineBlox.tsx so the
contract VERSION and the SDK EngineBlox.VERSION are committed atomically; keep
the existing safety logic (only allow commit when only the VERSION changes) but
apply it across both files.

In `@sdk/typescript/utils/metaTx/metaTransaction.tsx`:
- Around line 196-200: The domain for EIP-712 signing is using this.chain.id
instead of the chainId embedded in the unsigned payload; update the domain
construction in metaTransaction.tsx (where domain and META_TX_DOMAIN are used)
to set chainId from unsignedMetaTx.message.chainId (or the appropriate chain id
field on unsignedMetaTx) and keep verifyingContract as this.contractAddress so
signTypedData uses the same chain id that produced unsignedMetaTx.message,
preventing mismatched digests when the signer’s this.chain may be stale.

In `@test/foundry/helpers/TestHelpers.sol`:
- Around line 134-142: The test helper currently hardcodes the EIP‑712 version
string ("1.0.0") when building domainSeparator, which will desync from
MetaTxSigner.generateMessageHash/EngineBlox when the base version is bumped;
replace keccak256(bytes("1.0.0")) with a single shared constant reference (e.g.,
keccak256(bytes(PROTOCOL_VERSION)) or the same VERSION/PROTOCOL constant
exported by the contracts/SDK) so TestHelpers.sol uses the canonical version
value, and import or define that constant in TestHelpers.sol and use it in the
abi.encode call with DOMAIN_SEPARATOR_TYPE_HASH, PROTOCOL_NAME_HASH,
block.chainid and verifyingContract to keep byte-for-byte alignment.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/dependabot.yml:
- Line 6: Dependabot only targets the root manifest; fix by either (A) adding
Yarn/NPM workspaces to the root package.json (include the workspaces array
listing the subpackages) so Dependabot will pick up workspace manifests
automatically, then keep a single updates entry in .github/dependabot.yml
pointing at the root, or (B) add separate updates entries in
.github/dependabot.yml for each subpackage manifest (create an updates block per
package with package-ecosystem: "npm" and directory set to that subpackage) to
ensure docgen, package, and sdk/typescript manifests are updated; update the
config accordingly.

In `@scripts/sanity-sdk/guard-controller/base-test.ts`:
- Around line 643-649: The SDK isn't forwarding the explicit gas override so
eth_estimateGas still runs; update the write path in executeWriteContract
(BaseStateMachine.tsx) to extract the gas field from the passed options (in
addition to from, simulationMode, gasPrice) and include that gas value when
calling viem's writeContract; ensure callers like
roleConfigBatchRequestAndApprove (and getTxOptions) can pass a gas bigint
(1_500_000n) and it is propagated through executeWriteContract into the
writeContract invocation so the explicit gas limit prevents eth_estimateGas.

In `@scripts/sanity-sdk/secure-ownable/eip712-signing-tests.ts`:
- Around line 37-40: Change the untyped contract parameter and casts in
getOrCreateOwnershipTransferTxId to use the SecureOwnable type: replace the
parameter secureOwnableRecovery: any with secureOwnableRecovery: SecureOwnable
(the same SecureOwnable returned by createSecureOwnableWithWallet), and remove
(tx as any) and (receipt as any) casts—use the typed async call results and
their typed properties/methods (e.g., the typed transaction result fields and
receipt.status) exposed by SecureOwnable instead of unsafe casts; apply the same
typing fixes to other occurrences noted around lines 43–49 and 79–80.
- Around line 87-93: The code assumes the newly created transaction is the last
element of pendingAfter; instead, call
secureOwnableRecovery.getPendingTransactions() and filter for transactions with
type OWNERSHIP_TRANSFER and requester equal to the recovery wallet (same
filtering used earlier) after invoking transferOwnershipRequest, then locate the
matching transaction and return its ID; update the block that currently picks
pendingAfter[pendingAfter.length - 1] to search for the transaction by comparing
its type and requester (or other unique fields) and throw an error if no
matching OWNERSHIP_TRANSFER from the recovery wallet is found so the signing
flow never picks an unrelated pending transaction.
- Line 136: The assertion using boolean coercion on txId is wrong for bigint and
should be fixed: locate the call to this.assertTest(!!txId, 'Pending transaction
found for meta-tx signing') (the txId returned by
getOrCreateOwnershipTransferTxId) and either remove the redundant assertion
entirely (since getOrCreateOwnershipTransferTxId throws on error and returns a
bigint) or replace it with an explicit bigint check like this.assertTest(txId
!== 0n, 'Pending transaction found for meta-tx signing') if a zero value is
considered invalid; update only the assertion line so callers and types remain
consistent.

In `@scripts/sanity/guard-controller/base-test.cjs`:
- Around line 575-586: The timeout created for Promise.race in
sendTransactionWithValue is never cleared, which can keep the process alive;
modify sendTransactionWithValue to store the timer id when calling setTimeout
(e.g., let timer = setTimeout(...)) and ensure you clear it with
clearTimeout(timer) after the race completes (on both success and error paths) —
for example by awaiting Promise.race([sendPromise, timeoutPromise]) inside a
try/finally and calling clearTimeout(timer) in the finally block so the timer is
always removed.

In `@scripts/sanity/guard-controller/erc20-mint-controller-tests.cjs`:
- Around line 331-355: The ensureExactPermission function currently only
compares action bitmaps and repairs via addFunctionToRole, which reuses the
default self-referential handler mapping and can leave or reintroduce stale
handlerForSelectors, causing HandlerForSelectorMismatch; update
ensureExactPermission to also fetch and compare the handlerForSelectors for the
target function selector (using the same source as createFunctionPermission
creates), and when current handler differs from the desired handler call the
appropriate update (either pass the correct handler mapping into
addFunctionToRole or explicitly call the contract method that sets
handlerForSelector), ensuring you still call removeFunctionFromRole before
re-adding when repairing both bitmap and handler mismatches; reference
ensureExactPermission, addFunctionToRole, removeFunctionFromRole,
createFunctionPermission, and HandlerForSelectorMismatch when making the change.

In `@scripts/sanity/runtime-rbac/base-test.cjs`:
- Around line 479-494: The sendTransaction function leaves the setTimeout timer
alive because timeoutPromise is created with setTimeout that is never cleared;
update sendTransaction (the method using sendPromise, timeoutPromise) to capture
the timer id (e.g., let timer = setTimeout(...)) and ensure you
clearTimeout(timer) once Promise.race settles (both on success and on error) —
for example, create the timeout promise using the timer variable and then call
clearTimeout(timer) in a finally block or immediately after awaiting
Promise.race so no timers remain after method.send() resolves or rejects.

In `@scripts/sanity/secure-ownable/base-test.cjs`:
- Around line 310-329: The timeout created for the transaction receipt
(timeoutId from the sendTimeout Promise used with sendPromise returned by
method.send and receiptTimeoutMs) is not cleared on the error path; update the
try/catch so that if method.send() rejects you call clearTimeout(timeoutId)
inside the catch before building/throwing the new Error, ensuring the pending
timer is canceled even when sendPromise rejects.

In `@scripts/sanity/secure-ownable/eip712-signing-tests.cjs`:
- Around line 50-88: The test is only signing a synthetic hash and
manually-constructed txRecord so it doesn't exercise the contract EIP-712 path;
replace the manual construction and direct web3.eth.accounts.sign usage with the
contract-driven flow by calling generateUnsignedMetaTransactionForExisting(...)
to obtain the populated metaTx (so metaTx.message is set by the contract), then
pass that unsigned meta-transaction to eip712Signer.signMetaTransaction(...) to
produce the signature; ensure the test verifies the signature against the
contract-expected domain/types (META_TX_DOMAIN, META_TX_TYPES) and the selective
MetaTxRecord encoding (txId, params, payment) rather than a raw 0x.. message so
regressions in domain/type hashes or encoding are caught.

In `@scripts/sanity/utils/eip712-signing.cjs`:
- Around line 116-126: The generateMessageHash function currently accepts an
all-zero sentinel because _normalizeMessageHex treats it as valid; update
generateMessageHash (used by createSignedMetaTransaction) to detect and reject
the zero-placeholder by validating metaTx.message is not the zero hash (e.g.,
compare against the 32-byte zero hex like 0x000...000 or a regex /^0x0+$/ with
expected length) and throw a clear Error if detected so callers must supply the
contract-produced unsigned meta-transaction instead of signing the zero
placeholder.

---

Outside diff comments:
In `@scripts/sanity/guard-controller/base-test.cjs`:
- Around line 1853-1908: The helper ensureNativeTransferSchemaAndPermissions
currently only checks presence of schema/permission bits, which can leave
pre-split permission entries or incompatible bitmaps in place and cause
ResourceAlreadyExists in addFunctionToRole; update
ensureNativeTransferSchemaAndPermissions to normalize state for nativeSelector
by (1) when a role has any permission entry for nativeSelector, inspect the
existing bitmap and if it doesn't exactly match the desired signActions (for
ownerRoleHash) or execActions (for broadcasterRoleHash) remove or update that
function entry first (e.g., call the role/function removal or update helper
before adding) so the role ends up with exactly the expected bits, (2) handle
ResourceAlreadyExists from addFunctionToRole by attempting an idempotent
reconciliation (remove+re-add or update) rather than treating it as success, and
(3) use the existing helpers functionSchemaExists, registerFunction,
roleHasPermissionForSelector, addFunctionToRole when locating and normalizing
entries for nativeSelector to ensure the schema and both roles are in the
correct split sign/execute state.
- Around line 549-568: The code currently awaits
this.web3.eth.sendSignedTransaction(...) which can block until mining; instead,
trigger the transaction send and capture the transactionHash immediately by
using the PromiEvent events (e.g., call
this.web3.eth.sendSignedTransaction(signed.rawTransaction).once('transactionHash',
txHash => { ... }) or .on('transactionHash', ...)) rather than awaiting the
final promise—so in sendTransactionReceiptOnly replace the await on
sendSignedTransaction with subscribing to the 'transactionHash' event to obtain
transactionHash quickly (use signed.rawTransaction and store transactionHash),
then run the existing polling loop using receiptTimeoutMs to wait for the
receipt; also handle the sendSignedTransaction error event to surface send
errors.

In `@scripts/sanity/utils/eip712-signing.cjs`:
- Around line 91-106: The _normalizeMessageHex() helper is currently sanitizing,
truncating and padding inputs which can silently convert malformed digests into
different 32-byte values; change it to be strict: only accept a string that
already matches /^0x[0-9a-fA-F]{64}$/ and return null for everything else (do
not coerce bigint/other types, do not pad or truncate), and apply the same
strict behavior to the equivalent logic at lines ~181-189; ensure callers
generateMessageHash() and _signRawDigest() rely on this strict validation so
malformed digests are rejected instead of being rewritten and signed.

---

Nitpick comments:
In `@scripts/check-remote-ganache-health.ts`:
- Around line 122-139: The three independent client.readContract calls (for
functionName 'owner', 'getBroadcasters', and 'getRecovery' on accountBloxAddress
using accountBloxAbi) should be run in parallel rather than sequentially; change
the code to call Promise.all with the three client.readContract promises and
destructure the results into owner, broadcasters, and recovery, preserving
types/await usage and error handling around the combined Promise so the health
check behavior remains the same.
- Around line 66-86: The code currently casts values to Address without
validating format; update the logic around accountBloxAddress assignment to
validate parsed JSON value and process.env.ACCOUNTBLOX_ADDRESS using a proper
address checker (e.g., viem's isAddress) before assigning to accountBloxAddress:
when reading deployed.development.AccountBlox.address, call isAddress and only
set accountBloxAddress if valid (otherwise log a warning/error), and do the same
for the fallback from process.env.ACCOUNTBLOX_ADDRESS; keep the same console
messages but ensure invalid values are rejected rather than blindly cast.

In `@scripts/sanity-sdk/runtime-rbac/rbac-tests.ts`:
- Around line 695-713: Summary: duplicate error-message extraction across Steps
2,4,5,7,8 should be centralized and timeout handling normalized. Add a private
helper on the RuntimeRBACTests class named extractErrorMessage(error: unknown):
string that implements the current multi-source extraction (cause?.shortMessage
?? cause?.message ?? shortMessage ?? message ?? '').toString() safely, then
replace the inline extraction expressions in the catch blocks for Step 2, Step
4, Step 5 (use existing), Step 7 and Step 8 with calls to
this.extractErrorMessage(stepXError) and update the condition checks to test
both /Missing or invalid parameters/i and /took too long to respond/i so timeout
handling is consistent across all steps.

In `@sdk/typescript/utils/metaTx/metaTransaction.tsx`:
- Around line 65-66: The function buildTypedDataMessage currently returns
Record<string, unknown> and forces signTypedData through an as any cast around
lines ~204-210, erasing compile-time EIP-712 checks; define concrete EIP-712
types (e.g., EIP712Domain, MetaTransactionTypes and the MetaTransaction message
shape) and update buildTypedDataMessage(metaTx: MetaTransaction) to return the
precise typed payload (the TypedData type used by viem or equivalent generic:
domain, types, and message generics) so the signTypedData call can accept it
without any casts; remove the as any cast where signTypedData is invoked so viem
performs end-to-end type checking between buildTypedDataMessage, the
domain/types constants, and the signTypedData call.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@scripts/check-remote-ganache-health.ts`:
- Around line 145-149: The catch block currently logs "AccountBlox read failed
(owner/getBroadcasters/getRecovery reverted)" which incorrectly assumes a
revert; change the headline to a neutral message like "AccountBlox read failed"
(or "AccountBlox read failed (call error)") and include diagnostic details
(err.name, err.code, err?.message, and the existing data extraction) so callers
can distinguish revert vs bad address/ABI/RPC issues; keep the existing
extraction into the data variable (`const data: \`0x\${string}\` | undefined =
err?.data ?? err?.cause?.data ?? err?.cause?.cause?.data;`) and log it alongside
the error type/message.
- Around line 68-86: The code currently reads deployed-addresses.json first and
sets accountBloxAddress from deployed.development, which prevents the explicit
environment override from process.env.ACCOUNTBLOX_ADDRESS; change the lookup
order so the env var wins: first check process.env.ACCOUNTBLOX_ADDRESS and set
accountBloxAddress from it (logging the env source) before attempting to read
deployedPath/parse deployed.development, or if you prefer, after reading the
file explicitly prefer process.env.ACCOUNTBLOX_ADDRESS to override any value set
from the file; refer to the symbols accountBloxAddress, deployedPath,
deployed.development and process.env.ACCOUNTBLOX_ADDRESS when making the change.

In `@scripts/sanity-sdk/secure-ownable/eip712-signing-tests.ts`:
- Line 7: The import in eip712-signing-tests.ts imports TestWallet from the
wrong module; change the import so TestWallet is imported from the module that
actually exports it (the BaseSDKTest module) instead of './base-test.ts' —
update the import that currently reads "import { BaseSecureOwnableTest,
TestWallet } from './base-test.ts';" to import BaseSecureOwnableTest and
TestWallet from the module that defines BaseSDKTest/TestWallet.

In `@scripts/sanity/secure-ownable/base-test.cjs`:
- Around line 219-223: The test setup currently continues when
this.clearPendingTransactions() returns false, which masks real cleanup
failures; change the behavior to fail fast by treating a false return as an
error — in the same initialization/context where clearPendingTransactions is
called, replace the console.log fallback with throwing an Error (or calling
process.exit(1)) including a descriptive message like "Failed to clear pending
transactions: cannot guarantee clean on-chain state" so the suite aborts
immediately when this.clearPendingTransactions() returns false.

---

Duplicate comments:
In `@scripts/sanity-sdk/secure-ownable/eip712-signing-tests.ts`:
- Around line 37-53: The helper getOrCreateOwnershipTransferTxId is using
untyped any and positional fallbacks (secureOwnableRecovery: any, (tx as any),
params?.[4], params?.[0]) which is brittle; replace any with the correct
SecureOwnable SDK types and use the typed return shapes from
secureOwnableRecovery.getPendingTransactions() and getTransaction() (define or
import an interface for the transaction/result shape), then read named
properties (e.g., params.operationType and params.requester) instead of numeric
indexes; update the function signature and local variables to use those types so
the op and requester extraction no longer needs casting or positional fallbacks
(also apply the same typed fixes at the other occurrences around lines 87-88).

In `@scripts/sanity/guard-controller/base-test.cjs`:
- Around line 580-590: The timeout timer created in sendTransactionWithValue can
remain active if sendPromise rejects because clearTimeout(timeoutId) is only
called after a successful await; change the logic to always clear the timer
regardless of how Promise.race resolves by moving clearTimeout(timeoutId) into a
finally block (or add a .finally handler) around the await of
Promise.race([sendPromise, timeoutPromise]) so timeoutId is cleared on both
resolution and rejection.

In `@scripts/sanity/utils/eip712-signing.cjs`:
- Around line 116-123: The new zero-sentinel rejection in generateMessageHash
causes createSignedMetaTransaction -> signMetaTransaction to fail when callers
pass a zeroed message; instead of constructing a zero-placeholder message,
change the flow so createSignedMetaTransaction/signMetaTransaction receive the
contract-generated unsigned meta-tx that already has a populated message (use
generateUnsignedMetaTransactionForNew or
generateUnsignedMetaTransactionForExisting to obtain the populated metaTx and
thread that object through). Update any call sites of
createSignedMetaTransaction/signMetaTransaction to accept and forward the
unsigned meta-tx returned by the contract generator and remove/avoid creating a
'0x00...00' message placeholder so generateMessageHash can validate correctly.

---

Nitpick comments:
In `@scripts/check-remote-ganache-health.ts`:
- Around line 42-56: The script currently only logs
process.env.REMOTE_NETWORK_ID instead of enforcing it; add a real assertion that
compares Number(process.env.REMOTE_NETWORK_ID) to the fetched chainId (the
chainId value returned by client.getChainId()) and fail fast if they differ.
Locate the block using process.env.REMOTE_NETWORK_ID and replace the
logging-only behavior with a check that computes networkId =
Number(process.env.REMOTE_NETWORK_ID), compares networkId !== chainId, logs an
explicit error including both values via console.error or processLogger, and
exits non-zero (e.g., process.exit(1)) so the script fails when the RPC endpoint
is not the expected network. Ensure you still log the env value when it matches
for visibility.
- Around line 98-120: Replace the handwritten accountBloxAbi constant with the
canonical ABI from your build/artifact or typechain output: remove the inline
accountBloxAbi array and import the ABI (or use AccountBlox__factory.abi) from
the generated artifact/typechain so the script always uses the shared
AccountBlox ABI; update any code that referenced accountBloxAbi to use the
imported ABI symbol instead.

In `@scripts/sanity-sdk/secure-ownable/broadcaster-update-tests.ts`:
- Around line 95-100: Hoist the repeated numeric gas literal into a single
shared constant (e.g., DEFAULT_GAS = 500_000n) at the top of the test file and
replace each inline use of 500_000n in the getTxOptions calls (examples:
this.getTxOptions(ownerWallet.address, { gas: 500_000n }) used with
secureOwnableOwner.updateBroadcasterRequest and the other three occurrences)
with that constant so future tuning updates only touch one definition.

In `@scripts/sanity/guard-controller/base-test.cjs`:
- Around line 923-966: The duplicated normalization/decoding logic in
_callGenerateUnsignedMetaTransactionForExistingRaw (lines handling
tuple/msg/sig/dat/txRecord/params/messageHex and txRecord.message mutation)
should be extracted into a single helper (e.g., _normalizeUnsignedMetaOutput)
and used by both _callGenerateUnsignedMetaTransactionForExistingRaw and
_callGenerateUnsignedMetaTransactionForNewRaw; move the code that takes decoded
(or result) and derives tuple, msg, sig, dat, txRecord, params, computes
messageHex (padding/0x) and sets txRecord.message when appropriate into that
helper, have the helper return { txRecord, params, message, signature, data },
and replace the duplicated block in both functions with a call to the new helper
to ensure identical behavior and reduce drift.

In `@scripts/sanity/secure-ownable/base-test.cjs`:
- Around line 296-301: normalizeOperationType currently sanitizes and reshapes
malformed input which can hide ABI/decode mismatches; instead validate strictly
in normalizeOperationType: if operationType is null return null, else convert
non-string using this.web3.utils.toHex(operationType) only once, then assert the
result matches the exact bytes32 hex pattern /^0x[0-9a-fA-F]{64}$/ and return it
lowercased; if it does not match, throw a clear TypeError describing the invalid
operationType (referenced by normalizeOperationType) so callers like
getOperationName and cancelTransaction fail fast rather than silently rewriting
inputs.

In `@sdk/typescript/interfaces/base.index.tsx`:
- Line 17: The gas property currently accepts string but runtime validation in
BaseStateMachine.tsx only allows non-negative decimal digits; update the gas
declaration (gas?: number | bigint | string) to either narrow the type or, more
simply, add a short JSDoc on the gas field clarifying the accepted string format
(e.g., "string must be a non-negative decimal integer, digits only, no hex or
sign") and reference the runtime validator in BaseStateMachine so callers don't
pass unsupported formats.